Fix process handle leak on Windows #5270
Conversation
|
@swift-ci test |
|
I see the process tests are skipped on windows for a reason that as since been fixed... going to see If I can get then going again... |
daveinglis
force-pushed
the
fix_handle_leak
branch
3 times, most recently
from
eb655ca to
a79623a
Compare
|
@swift-ci test |
|
@swift-ci test windows |
daveinglis
force-pushed
the
fix_handle_leak
branch
2 times, most recently
from
ca97bc0 to
b3edb46
Compare
|
@swift-ci test |
|
@swift-ci test windows |
|
@swift-ci test window |
|
@swift-ci test windows |
daveinglis
force-pushed
the
fix_handle_leak
branch
from
b3edb46 to
b8ad565
Compare
|
@swift-ci test |
|
@swift-ci test windows |
daveinglis
force-pushed
the
fix_handle_leak
branch
from
b8ad565 to
9090ee9
Compare
|
@swift-ci test |
|
@swift-ci test windows |
daveinglis
force-pushed
the
fix_handle_leak
branch
from
9090ee9 to
d6447a6
Compare
|
@swift-ci test |
There was a problem hiding this comment.
This looks good to me; please get @iCharlesHu or @parkera to review as well.
|
@swift-ci test Windows |
1 similar comment
|
@swift-ci test Windows |
|
@weissi requested changes have been made. @iCharlesHu and/or @parkera, Can this be merged? |
| closeHandler() | ||
| } else { | ||
| closeHandler() | ||
| } | ||
|
|
||
| func closeHandler() { | ||
| #if os(Windows) | ||
| CloseHandle(self.processHandle) | ||
| #endif |
There was a problem hiding this comment.
I'm not sure I understand why this structure is needed as opposed to:
| closeHandler() | |
| } else { | |
| closeHandler() | |
| } | |
| func closeHandler() { | |
| #if os(Windows) | |
| CloseHandle(self.processHandle) | |
| #endif | |
| } | |
| #if os(Windows) | |
| CloseHandle(self.processHandle) | |
| #endif |
Perhaps having a comment would be helpful?
There was a problem hiding this comment.
@jakepetroules mentioned (in a previous change) that this should be called as late as possible, even after any terminateHandler (which runs in its own thread) hence the reason for why its like this. I will add a comment.
- Add a missing CloseHandle as part of the process termination handling - Enable Process tests on Windows
daveinglis
force-pushed
the
fix_handle_leak
branch
from
d6447a6 to
a44e88a
Compare
|
@swift-ci test |
|
@swift-ci test windows |
|
@iCharlesHu can you review this please? |
6 participants
Uh oh!
There was an error while loading. Please reload this page.